Skip to content

feat(access): let a grant open a UI surface, not only reach data - #899

Open
jirhiker wants to merge 1 commit into
feat/scope-based-access-controlfrom
feat/ui-surface-grants
Open

feat(access): let a grant open a UI surface, not only reach data#899
jirhiker wants to merge 1 commit into
feat/scope-based-access-controlfrom
feat/ui-surface-grants

Conversation

@jirhiker

Copy link
Copy Markdown
Member

What

A permission grant could reach data. Now it can also open a screen.

ui_surface names a navigation item or page in the admin UI — ocotillo.lexicon, ocotillo.location, and so on — using the same resource identifier the UI already checks in canAccessResource, so a lexicon term and a nav item cannot drift apart.

// a data grant, unchanged
{ "capability": "read", "data_type": "water level", "scope_type": "thing", "scope_id": 7 }

// a screen grant
{ "capability": "read", "ui_surface": "ocotillo.lexicon", "scope_type": "global" }

Based on feat/scope-based-access-control, not staging — it needs the ADR5 access tables and the list-all-grants route.

The no-wildcard rule is kept, not relaxed

data_type becomes nullable, which looks like a loosening. It is not. The rule it carried — a grant names its subject, and there is no term meaning "all" — is replaced by a strictly equivalent one:

Exactly one of data_type / ui_surface is set. Never neither, never both.

  • Neither is the old MissingDataType, unchanged in spirit: a blanket grant is what published data nobody agreed to publish.
  • Both is a new AmbiguousGrantSubject. A row naming both would be two grants wearing one revocation, and revoking the data half would silently take the screen away too. Write two grants so each can be revoked on its own.

The XOR is enforced in domain/access.py before any row is written, not as a check constraint, for the reason the scope rule is: so it holds for every writer and reads as one sentence.

A surface grant is always global

New ScopedSurfaceGrant. Navigation is app-wide — the UI asks "may this caller see this screen", never "for this well" — so a thing- or group-scoped surface grant could not match any request the UI makes. It is refused at the door rather than stored as a row that silently never applies.

Matching never treats None as a match

A data grant and a screen request both carry None on the axis not being asked about. Matching on None == None would make a data grant answer a screen question, so _subject_matches requires the asked axis to be present and equal. A request naming neither subject is a no — default deny, because a question this layer cannot answer is not a yes.

Surface area

Route Change
POST /access/grant accepts ui_surface; 422 on both-or-neither, and on a scoped surface grant
GET /access/grant new optional ui_surface filter
GET /access/decision accepts ui_surface; data_type now optional; 422 if both are asked at once

Plus services.visibility.may_see_surface, a thin reading of may — surface grants are always global and always read, so callers do not restate either.

Widen-only by construction. This answers whether a grant opens a screen. The UI falls back to its role policy when the answer is no, so a missing grant can never take away what a role already allows, and a grants outage cannot lock an admin out.

Migration

a396d7d9928d. Adds ui_surface (nullable, FK to lexicon_term), makes data_type nullable. Existing rows all carry a data_type and are untouched.

The downgrade deletes surface grants before restoring NOT NULL. That is the honest direction: they are grants the old schema has no way to express, and giving them an invented data_type would grant data access nobody asked for. Both directions were run against a live database, with a surface grant present, and the row was removed as documented.

ui_surface is a lexicon category seeded from core/lexicon.json, not an enum type, so adding a screen later is not a migration.

Verification

  • tests/test_domain_access.py — 46 pass, 13 new: the XOR, the global-only rule, None-is-not-a-match in both directions, and that a surface grant expires and revokes like any other.
  • tests/test_access.py — 36 pass, 11 new, through the routes: grant, filter, decide, revoke, and both 422s.
  • Full suite: 1475 passed, 84 skipped, 6 xpassed. The 3 failures in tests/transfers/test_contact_with_multiple_wells.py are a missing local gcs_credentials.json pointing into another checkout — unrelated, and failing the same way before this branch.
  • black clean. flake8 reports only E501, which the repo already produces on these files untouched (black targets 88, there is no flake8 config).

One defect this caught in itself, worth noting: the first cut passed a bare Enum into the 422 detail, which is not JSON serializable. Fixed, and the route test that found it is in the suite.

Consumer

DataIntegrationGroup/OcotilloUI#365 adds the access console. The UI side of surface grants goes on top of this.

A permission grant could name a data type. It can now name a `ui_surface`
instead: a screen or navigation item in the admin UI, so "this person may
see the Lexicon editor" is a grant rather than a role change in Authentik.

Surfaces are named by the resource identifier the UI already checks
(`ocotillo.lexicon`, `ocotillo.thing-well`, ...), so a lexicon term and a
nav item cannot drift apart. They are lexicon terms, not an enum type, so
adding a screen later is a seed, not a migration.

## The no-wildcard rule is kept, not relaxed

`data_type` becomes nullable to make room for `ui_surface`, which reads
like a loosened invariant and is not one. A grant still names exactly one
subject: the XOR between the two columns is enforced in domain/access.py
before any row is written, so it holds for every writer rather than only
for the route. Neither vocabulary has a term meaning "all", so a data type
or a screen added next year is still never covered by an existing grant.

A grant naming both is refused. It would be two grants wearing one
revocation, and revoking the data half would silently take the screen away
too — write two grants so each can be revoked on its own.

## A surface grant is always global

Navigation is app-wide: the UI asks "may this caller see this screen",
never "for this well". A group- or thing-scoped surface grant could not
match any request the UI makes, so it is refused at the door rather than
stored as a row that silently never applies.

## Matching

`_subject_matches` never matches an unasked axis. Both a data grant and a
surface request carry `None` on the axis the other names, and matching on
`None == None` would have made a water-level grant answer "may I see the
Lexicon editor" with yes.

`may()` takes `ui_surface` alongside `data_type`, and `may_see_surface()`
is the thin reading of it the UI will use. A call naming neither subject is
a no, not an error: a question this layer cannot answer is not a yes.

`GET /access/decision` refuses a call naming both — two questions have two
answers — and `GET /access/grant` filters by `ui_surface`.

## Downgrade

A surface grant has no data_type to fall back to, so it cannot survive the
column being NOT NULL again. The downgrade deletes those rows, which is the
honest reading: they are grants the old schema cannot express, and giving
them an invented data_type would grant data access nobody asked for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Your pull request is automatically being deployed to Dagster Cloud.

Location Status Link Updated
ocotillo-automated-ingestion View in Cloud Aug 28, 2026 at 04:34 PM (UTC)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant